-
Notifications
You must be signed in to change notification settings - Fork 528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[create-vsix] Create .vsix
files
#541
Conversation
TODO: Double-check how the XamarinVS repo creates the |
c390259
to
3513aa6
Compare
<Installation AllUsers="true"> | ||
<InstallationTarget Version="[15.0,)" Id="Microsoft.VisualStudio.Community" /> | ||
</Installation> | ||
<Dependencies> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this node altogether. It's not needed.
<?xml version="1.0" encoding="utf-8"?> | ||
<PackageManifest Version="2.0.0" xmlns="http://schemas.microsoft.com/developer/vsx-schema/2011" xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011"> | ||
<Metadata> | ||
<Identity Id="4489ebd9-10e4-42d0-bc3c-0639c608aaea" Version="1.0" Language="en-US" Publisher="Company" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make these match the values in https://github.com/xamarin/XamarinVS/blob/master/src/Installer/Android.Sdk/source.extension.vsixmanifest#L4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note how the version can be fetched dynamically by invoking an MSBuild target that can read it from wherever: |Android.Sdk;GetVsixVersion|
(that's |PROJECT_NAME;TARGET_NAME|
)
4db94ef
to
6b0dd12
Compare
<_BuildVsix Condition=" '$(_BuildVsix)' == '' ">False</_BuildVsix> | ||
</PropertyGroup> | ||
<Import Project="..\..\Configuration.props" /> | ||
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that no code will ever run from the VSIX itself, how about just removing entirely the two property groups that are conditioned and just add one property below _BuildVsix
as <OutputPath>bin\$(Configuration)</OutputPath>
and just declare the two conditioned property groups empty just to satisfy VS?
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' " />
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' " />
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Historically, Xamarin Studio gets rather cross if It doesn't have an explicit Debug property group and Release property group. I haven't tried in this case, but odds are against me.
<WarningLevel>4</WarningLevel> | ||
<ConsolePause>false</ConsolePause> | ||
</PropertyGroup> | ||
<ItemGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ItemGroup (and the AssemblyInfo.cs itself) shouldn't be needed if we aren't including any built outputs?
--> | ||
<Project InitialTargets="RedirectMonoAndroidSdkPaths;RemoveSdksCache" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
|
||
<PropertyGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole file was intended only to work on VS2017+, which is true as long as '$(VsInstallRoot)' != ''
. Maybe the property groups and targets in this file should have the condition, just to make it more explicit and avoid unintentional usage outside of that scenario?
e43508e
to
fa69fd4
Compare
6836f6c
to
69d1473
Compare
ff74feb
to
7c57727
Compare
Visual Studio 2017 uses `.vsix` files for Xamarin.Android SDK support. A `.vsix` file is [ZIP container with additional metadata][0], and the [Microsoft.VSSDK.BuildTools NuGet package][1] contains various MSBuild targets and tools to assist in creating `.vsix` files. Add a new `build-tools/create-vsix/create-vsix.csproj` project to create the `bin/Build$(Configuration)/Xamarin.Android.Sdk*.vsix` file, so that we can plausibly provide per-build OSS Xamarin.Android releases that work with Visual Studio 2017. Unfortunately those tools were written on Windows, and not really well tested on macOS or Linux... In particular, there are case-sensitivity and directory-separator-char issues with the tooling, necessitating that `MONO_IOMAP=all` be exported in order to run them: MONO_IOMAP=all MONO_OPTIONS=--arch=64 msbuild \ build-tools/create-vsix/create-vsix.csproj /p:CreateVsixContainer=True Meanwhile, we're still attempting to allow things to be built with `xbuild` [^3]. Thread this needle by "special-casing" the `$(BuildDependsOn)`, `$(CopyVsixManifestFileDependsOn)`, and `$(DetokenizeVsixManifestFileDependsOn)` MSBuild properties so that when the `$(CreateVsixContainer)` MSBuild property is False -- the default -- no `.vsix` package will be created, and `xbuild` will be able to build the project. Similar needle threading is needed to "support" building on Linux: the Microsoft.VSSDK.BuildTools NuGet package uses case in an inconsistent fashion -- or is it `nuget`? -- which results in [failures when building on Linux][2] because `nuget` extracts e.g.: packages/Microsoft.VSSDK.BuildTools.15.0.26201/tools/vssdk/Microsoft.VsSDK.targets while Microsoft.VSSDK.BuildTools attempts to `<Import/>` the file: packages/Microsoft.VSSDK.BuildTools.15.0.26201/tools/VSSDK/Microsoft.VsSDK.targets `vssdk` != `VSSDK` on case-sensitive filesystems, so this results in an error on Ubuntu (and presumably case-senstive macOS as well). Handle the Linux case by extending the `xbuild` case: *even when* `$(CreateVsixContainer)` is True, we won't actually build the `.vsix` unless the `$(VsSDKInstall)` directory exists, which is the `tools/VSSDK` path. Building with `$(CreateVsixContainer)` set to True will require using `msbuild` with `MONO_IOMAP=all` and `MONO_OPPTIONS=--arch=64` exported, while using a case-insensitive filesystem. The new `make create-vsix CONFIGURATIONS=Debug` target can be used to explicitly create the `Xamarin.Android.Sdk*.vsix` file. One problem with creating `.vsix` files: macOS still defaults to using a 32-bit process for `mono`, and `make create-vsix` regularly fails for me with an `OutOfMemoryException` when attempting to generate a `.vsix` file ~600-700+MB in size. (Why so large? In part because for Debug configuration we're including un-`strip`'d native libraries; `libmonosgen-2.0.dll` is *237MB* in size (!); it's closer to 5MB when `strip`'d, but any un-`strip`'d MXE-generated native libraries to the `.vsix` file quickly results in `OutOfMemoryException`s.) A 64-bit mono can be used by using `mono64` -- which isn't easily done with `msbuild` -- or by using `mono --arch=64`, which *can* be done with `msbuild` by exporting `MONO_OPTIONS=--arch=64`. The `make create-vsix` target does this. Finally, once we have a `.visx` being created, we then examine the *contents* of the `.vsix` file, which is...weird: $MSBuild/Xamarin/Android/Xamarin.Android.CSharp.targets # Desirable $ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v1.0/Xamarin.Android.CSharp.targets # wat? $ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v1.0/Facades/Xamarin.Android.CSharp.targets # really?! $ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v7.1/Xamarin.Android.CSharp.targets # !!!! Turns Out™, the problem was due to `@(ProjectReference)`. We're (ab)using `@(ProjectReference)` to explicitly specify project dependencies, and the (implicit) project build order. Unexpectedly (forgetten?), MSBuild *also* copies the *outputs* of `@(ProjectReference)`s into the `$(OutputPath)` of the current project. Since e.g. `Mono.Posix.csproj` had a reference on `Xamarin.Android.Build.Tasks.csproj`, the result of this is that the `$(OutputPath)` of `Mono.Posix.csproj` -- `$prefix/lib/xbuild-frameworks/MonoAndroid/v1.0` -- also contained the `%(None.CopyToOutputDirectory)` items from `Xamarin.Android.Build.Tasks.csproj`. Oops. The fix is to set `%(ProjectReference.Private)` to False, which disables this copying of additional and undesirable files. [0]: https://msdn.microsoft.com/en-us/library/dd997148.aspx [1]: https://www.nuget.org/packages/Microsoft.VSSDK.BuildTools/ [2]: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-anroid-linux-pr-builder/297/consoleText [^3]: But for how much longer?
Changes: dotnet/java-interop@f3553f4...d61b329 * dotnet/java-interop@d61b329 [Java.Interop-Tests] Relax Exception.Message checks (dotnet#544) * dotnet/java-interop@d778204 [Java.Interop] Better linker-friendly JavaObjectArray<T>.ValueMarshaler (dotnet#546) * dotnet/java-interop@4bd07e0 [vs-code] Recommend the zbecknell.t4-support extension (dotnet#545) * dotnet/java-interop@3b24a2c [Java.Interop] apply AggressiveInlining where profiler shows calls (dotnet#541)
Changes: dotnet/java-interop@f3553f4...d61b329 * dotnet/java-interop@d61b329 [Java.Interop-Tests] Relax Exception.Message checks (#544) * dotnet/java-interop@d778204 [Java.Interop] Better linker-friendly JavaObjectArray<T>.ValueMarshaler (#546) * dotnet/java-interop@4bd07e0 [vs-code] Recommend the zbecknell.t4-support extension (#545) * dotnet/java-interop@3b24a2c [Java.Interop] apply AggressiveInlining where profiler shows calls (#541)
Visual Studio 2017 uses
.vsix
files for Xamarin.Android SDK support.A
.vsix
file is ZIP container with additional metadata, and theMicrosoft.VSSDK.BuildTools NuGet package contains various MSBuild
targets and tools to assist in creating
.vsix
files.Add a new
build-tools/create-vsix/create-vsix.csproj
project tocreate the
bin/Build$(Configuration)/Xamarin.Android.Sdk*.vsix
file,so that we can plausibly provide per-build OSS Xamarin.Android
releases that work with Visual Studio 2017.
Unfortunately those tools were written on Windows, and not really well
tested on macOS or Linux... In particular, there are case-sensitivity
and directory-separator-char issues with the tooling, necessitating
that
MONO_IOMAP=all
be exported in order to run them:Meanwhile, we're still attempting to allow things to be built with
xbuild
1. Thread this needle by "special-casing" the$(BuildDependsOn)
,$(CopyVsixManifestFileDependsOn)
, and$(DetokenizeVsixManifestFileDependsOn)
MSBuild properties so thatwhen the
$(CreateVsixContainer)
MSBuild property is False -- thedefault -- no
.vsix
package will be created, andxbuild
will beable to build the project.
Similar needle threading is needed to "support" building on Linux: the
Microsoft.VSSDK.BuildTools NuGet package uses case in an inconsistent
fashion -- or is it
nuget
? -- which results infailures when building on Linux because
nuget
extracts e.g.:while Microsoft.VSSDK.BuildTools attempts to
<Import/>
the file:vssdk
!=VSSDK
on case-sensitive filesystems, so this results inan error on Ubuntu (and presumably case-senstive macOS as well).
Handle the Linux case by extending the
xbuild
case: even when$(CreateVsixContainer)
is True, we won't actually build the.vsix
unless the
$(VsSDKInstall)
directory exists, which is thetools/VSSDK
path.Building with
$(CreateVsixContainer)
set to True will require usingmsbuild
withMONO_IOMAP=all
andMONO_OPPTIONS=--arch=64
exported, while using a case-insensitive filesystem.
The new
make create-vsix CONFIGURATIONS=Debug
target can be used toexplicitly create the
Xamarin.Android.Sdk*.vsix
file.One problem with creating
.vsix
files: macOS still defaults to usinga 32-bit process for
mono
, andmake create-vsix
regularly failsfor me with an
OutOfMemoryException
when attempting to generate a.vsix
file ~600-700+MB in size. (Why so large? In part because forDebug configuration we're including un-
strip
'd native libraries;libmonosgen-2.0.dll
is 237MB in size (!); it's closer to 5MB whenstrip
'd, but any un-strip
'd MXE-generated native libraries to the.vsix
file quickly results inOutOfMemoryException
s.)A 64-bit mono can be used by using
mono64
-- which isn't easily donewith
msbuild
-- or by usingmono --arch=64
, which can be donewith
msbuild
by exportingMONO_OPTIONS=--arch=64
.The
make create-vsix
target does this.Finally, once we have a
.visx
being created, we then examine thecontents of the
.vsix
file, which is...weird:Turns Out™, the problem was due to
@(ProjectReference)
. We're(ab)using
@(ProjectReference)
to explicitly specify projectdependencies, and the (implicit) project build order.
Unexpectedly (forgetten?), MSBuild also copies the outputs of
@(ProjectReference)
s into the$(OutputPath)
of the currentproject.
Since e.g.
Mono.Posix.csproj
had a reference onXamarin.Android.Build.Tasks.csproj
, the result of this is that the$(OutputPath)
ofMono.Posix.csproj
--$prefix/lib/xbuild-frameworks/MonoAndroid/v1.0
-- also contained the%(None.CopyToOutputDirectory)
items fromXamarin.Android.Build.Tasks.csproj
.Oops.
The fix is to set
%(ProjectReference.Private)
to False, whichdisables this copying of additional and undesirable files.
Footnotes
But for how much longer? ↩